-
-
Notifications
You must be signed in to change notification settings - Fork 291
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor(network): move unclean disconnect log to debug #5541
Conversation
@@ -628,7 +628,7 @@ export class PeerManager { | |||
try { | |||
await this.libp2p.hangUp(peer); | |||
} catch (e) { | |||
this.logger.warn("Unclean disconnect", {peer: prettyPrintPeerId(peer)}, e as Error); | |||
this.logger.debug("Unclean disconnect", {peer: prettyPrintPeerId(peer)}, e as Error); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Anyone knows why prettyPrintPeerId
only prints out the first 2 characters? Those seem to be always 16 , e.g. peer=16...LaZW3U
.
I saw users wonder why peer 16 is always disconnecting 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah that's a good question. For this context I suppose we should print the whole peer id as it's for developer, not for user. We can ask other client for a specific peer id for example
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moving this from warn to debug will solve most of the confusion for users as most do not look at debug logs. We are using prettyPrintPeerId
all over the place in peer manager, maybe could print first 4 characters instead of just 2.
We can ask other client for a specific peer id for example
Not sure how they would look up the peer id but would assume if it just based on searching the logs it should even be sufficient to just have the truncated peer id.
Anyways, changes on how we want to log the peer ids should probably be addressed in another PR.
Performance Report✔️ no performance regression detected Full benchmark results
|
🎉 This PR is included in v1.9.0 🎉 |
Motivation
#5359 (comment)
Description
Move unclean disconnect log to from
warn
todebug